Skip to content

[v7r2] X509Request: correct comparison of public keys#4863

Merged
atsareg merged 1 commit into
DIRACGrid:integrationfrom
chaen:rel-v7r2_FIX_pubKey
Dec 9, 2020
Merged

[v7r2] X509Request: correct comparison of public keys#4863
atsareg merged 1 commit into
DIRACGrid:integrationfrom
chaen:rel-v7r2_FIX_pubKey

Conversation

@chaen

@chaen chaen commented Dec 9, 2020

Copy link
Copy Markdown
Contributor

Renable the test disabled here #4858
The real problem was a incorrect test in the code, exposed by a change of behaviour of openssl.

BEGINRELEASENOTES

*Core
FIX: Correct comparison of public key for X509Request

ENDRELEASENOTES

For the curious (and my memory in the future), details here

import M2Crypto
import six

# Dirty hack... in fact, it's just that our python2 environments
# uses openssl 1.0.2u and the python 3 openssl 1.1.1h
# and that's the real difference.
IS_OPENSSL_1_1_1 = six.PY3

# Generate a pair of keys and assign it to a Request
rsa = M2Crypto.RSA.gen_key(1024, 65537, callback=M2Crypto.util.quiet_genparam_callback)
pkeyObj = M2Crypto.EVP.PKey()
pkeyObj.assign_rsa(rsa)
reqObj = M2Crypto.X509.Request()
reqObj.set_pubkey(pkeyObj) 

reqObj.sign(pkeyObj, "sha256")

assert reqObj.verify(pkeyObj)

# Dump the request as PEM
reqStr = reqObj.as_pem()

# Reload the request as a new object

reqObj2 = M2Crypto.X509.load_request_string(reqStr, format=M2Crypto.X509.FORMAT_PEM)

print("pkeyObj.as_pem")
print(pkeyObj.as_pem(cipher=None).decode())

print()

print("reqObj.get_pubkey().as_pem")
print(reqObj.get_pubkey().as_pem(cipher=None).decode())

print()

print("reqObj2.get_pubkey().as_pem")
print(reqObj2.get_pubkey().as_pem(cipher=None).decode())

# Compare the PEM dump of the Request objects
assert reqObj.as_pem() == reqObj2.as_pem()

# get the public key of the RSA key
bio = M2Crypto.BIO.MemoryBuffer()
rsa.save_pub_key_bio(bio)
rsa_pub= bio.read()         


# get_rsa returns the public RSA key
# so we are comparing the public key of
# the RSA key 
# the EVP
# the first request object
# the second request object
assert reqObj.get_pubkey().get_rsa().as_pem(cipher=None) \
       == reqObj2.get_pubkey().get_rsa().as_pem(cipher=None) \
       == rsa_pub \
       == pkeyObj.get_rsa().as_pem(cipher=None)


# Here we compare a der dump of the public key of the request
# and the EVP object. As the DER dump dumps only the public
# part, they should be the same
assert reqObj.get_pubkey().as_der() == pkeyObj.as_der() == reqObj2.get_pubkey().as_der()

# That should NOT be the same, since 
# the pem dump dumps private keys info. 
# in the case of the request, there are no private keys
# BUT it seems like with openssl 1.1.1, the private key comes along
# It seems to be due to that: https://github.com/openssl/openssl/commit/fa0a9d715e7e35d4f597683c16b643343245fa26#diff-7ccb70ea343c7e0cdcb92d9171980816e5a4d19f56e6d4e707bc396c581e8868R135
if IS_OPENSSL_1_1_1:
  assert reqObj.get_pubkey().as_pem(cipher=None) == pkeyObj.as_pem(cipher=None)
else:
  assert reqObj.get_pubkey().as_pem(cipher=None) != pkeyObj.as_pem(cipher=None)

# Compare the PEM dump of their keys
# as_pem dumps private info.
# the EVP of the public key should contain rubish
# so they SHOULD be different. But they are not with old version
# Doing some more tests, I have the feeling that the private part is initialized
# with something related to the RSA private key, but it is not the key itself.
# Anyway, this comparison DOES NOT MAKE SENSE, and this is what was done in DIRAC
if IS_OPENSSL_1_1_1:
  assert reqObj.get_pubkey().as_pem(cipher=None) != reqObj2.get_pubkey().as_pem(cipher=None)
else:
  assert reqObj.get_pubkey().as_pem(cipher=None) == reqObj2.get_pubkey().as_pem(cipher=None)

@chaen chaen requested review from atsareg and fstagni as code owners December 9, 2020 15:41
@atsareg

atsareg commented Dec 9, 2020

Copy link
Copy Markdown
Contributor

closed to rerun tests

@atsareg atsareg closed this Dec 9, 2020
@atsareg atsareg reopened this Dec 9, 2020
@atsareg atsareg merged commit af86e5a into DIRACGrid:integration Dec 9, 2020
@chaen chaen deleted the rel-v7r2_FIX_pubKey branch August 11, 2022 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants